feat: Added the ability to filter the corresponding files based on the access_time, modify time, and change time of the file for statistics. #399

wugeer · 2024-05-17T07:51:45Z

Background

Filter by file age #340
Add a new feature to dust to filter files based on their last update time, last access time, and last modification time.

In order to achieve this requirement, I referred to the input of the linux find command, and ultimately planned to implement the dust filter by file age function based on the input style of find.

I have a little question here, why does dust output files or directories with a file size of 0 by default? The dust tool is used to quickly find those files and directories in the specified directory that take up too much space, and those empty files or directories will only affect the look and feel. Originally, I have implemented filtering out those files or directories with a size of 0, but when I run cargo test When running the test, I found that some samples reported errors, so I canceled this part of the code, but I still wanted to retain the function of filtering out those files or directories with a size of 0. I hope this function can be further explored. thanks

Full Changelogs

feat: Added the ability to filter the corresponding files based on the access_time, modify time, and change time of the file for statistics

Issue Reference

fix: Filter by file age #340

Test Result

dust help command output

Test Data

cargo test run test results

Adding --mtime or --atime or --ctime results in actual execution of dust and linux find with similar parameters.

bootandy · 2024-05-21T21:43:01Z

Apologies I've had a laptop failure. Don't want to risk looking at this on my work laptop for legal reasons. Will review this when I get a new machine.

codeswhite · 2024-05-26T22:32:42Z

completions/_dust

@@ -37,6 +37,12 @@ _dust() {
 '--output-format=[Changes output display size. si will print sizes in powers of 1000. b k m g t kb mb gb tb will print the whole tree in that size.]:FORMAT:(si b k m g t kb mb gb tb)' \
 '-S+[Specify memory to use as stack size - use if you see\: '\''fatal runtime error\: stack overflow'\'' (default low memory=1048576, high memory=1073741824)]:STACK_SIZE: ' \
 '--stack-size=[Specify memory to use as stack size - use if you see\: '\''fatal runtime error\: stack overflow'\'' (default low memory=1048576, high memory=1073741824)]:STACK_SIZE: ' \
+'-M+[File'\''s data was last modified less than, more than or exactly <mtime>*24 hours ago. When dust figures out how many 24-hour periods ago the file was last modified, any fractional part is ignored, so to match -mtime +1, a file has to have been modified at least two days ago.]: : ' \


Consider making the description more easily readable

Suggested change

'-M+[File'\''s data was last modified less than, more than or exactly <mtime>*24 hours ago. When dust figures out how many 24-hour periods ago the file was last modified, any fractional part is ignored, so to match -mtime +1, a file has to have been modified at least two days ago.]: : ' \

'-M+[Sort by time modifed, include only files modified +/-<mtime> days before/after, days are rounded down, prefix \'\-\' for newer, \'\+\' for older. : : ' \

In my opinion, Filter by time mofied is better than Sort by time modifed because this parameter is used for filtering, not sorting. right? :)

Sure, you can rephrase it as you wish, just pointed that its a bit too long :)

Indeed, that description is a bit too long. I rephrased it . I will be very happy if you can help me review the code again. :)

codeswhite · 2024-05-27T20:50:08Z

src/dir_walker.rs

@@ -167,6 +191,9 @@ fn walk(dir: PathBuf, walk_data: &WalkData, depth: usize) -> Option<Node> {
                                        data.is_file(),
                                        walk_data.by_filecount,
                                        depth,
+                                        filter_modified_time,


Consider changing build_node to accept a walk_data argument instead of individually 7 of walk_data's fields

Indeed, you are right, I reorganized the code according to your ideas. I will be very happy if you can help me review the code again. :)

rindeal · 2024-05-29T02:29:43Z

@bootandy: Apologies I've had a laptop failure. Don't want to risk looking at this on my work laptop for legal reasons. Will review this when I get a new machine.

You are always one click away from a GitHub Codespace

codeswhite

Looks good!
Should wait for @bootandy to review as well

I still suggest that after merging this we continue to work on a more robust implementation that will support all timeframes (like i explained in #340 )

wugeer · 2024-05-31T03:46:15Z

Looks good! Should wait for @bootandy to review as well

I still suggest that after merging this we continue to work on a more robust implementation that will support all timeframes (like i explained in #340 ) All timeframes (like I explained in #340).

That sounds like a good idea. I like it! I'll try it. ：)

wugeer · 2024-06-11T08:01:11Z

Looks good! Should wait for @bootandy to review as well

I still suggest that after merging this we continue to work on a more robust implementation that will support all timeframes (like i explained in #340 )

I've been thinking seriously about this over the past few days. If we modify the tool according to your approach, we'll need to add a parameter to set the upper and lower limits of the filter time. Otherwise, the scenario for equal values would no longer exist. While this method provides great flexibility, I'm concerned about whether such a high level of flexibility is necessary for this tool. Adding this feature might place a significant cognitive burden on the users. After all, people are probably accustomed to using the find command with the -mtime option, which filters by days, or the -mmin option, which filters by minutes. What are your thoughts on this? @bootandy

bootandy · 2024-06-17T20:27:32Z

I have a little question here, why does dust output files or directories with a file size of 0 by default?

Just incase you are running on a very small folder. Some people seem to use dust as a sort of file-finder so I want to include them.

bootandy · 2024-06-17T20:32:43Z

completions/_dust.ps1

+            [CompletionResult]::new('-A', 'A ', [CompletionResultType]::ParameterName, 'just like -mtime, but based on file access time')
+            [CompletionResult]::new('--atime', 'atime', [CompletionResultType]::ParameterName, 'just like -mtime, but based on file access time')
+            [CompletionResult]::new('-y', 'y', [CompletionResultType]::ParameterName, 'just like -mtime, but based on file change time')
+            [CompletionResult]::new('--ctime', 'ctime', [CompletionResultType]::ParameterName, 'just like -mtime, but based on file change time')


I'm not sure if we need all these - I like the idea of a including a filter that works with modified time. But 'access time' - would someone actually use that? I have to launch 'stat' to even see it.

Ah we can leave it in if you'd like.

In my opinion, access time is an effective filtering tool when users want to determine whether a file has been accessed recently based on the access time to determine whether other people are using the file.

bootandy

I like that you based the parameters for this work from find.

We seem to have a few cargo style problems. But overall I think this approach is correct.

bootandy · 2024-06-17T20:34:50Z

src/dir_walker.rs

                                        depth,
+                                        walk_data,


sensible change, there were too many params.

wugeer · 2024-06-18T00:57:44Z

I have a little question here, why does dust output files or directories with a file size of 0 by default? I have a little question, why does dust output files or directories with a file size of 0 by default?

Just incase you are running on a very small folder. Some people seem to use dust as a sort of file-finder so I want to include them. Some people seem to use dust as a file finding tool, so I wanted to include them.

Yes, you are right! :)

wugeer · 2024-06-18T01:03:25Z

I like that you based the parameters for this work from find. I like that you based the parameters for this work from find.

We seem to have a few cargo style problems. But overall I think this approach is correct. We seem to have a few cargo style problems. But overall, I think this approach is the right one.

I'm fairly new to Rust , so my coding style may not be very good. If there is anything that needs to be modified, I hope you will give me some advice. thx!

bootandy · 2024-06-18T22:22:39Z

I'm fairly new to Rust , so my coding style may not be very good. If there is anything that needs to be modified, I hope you will give me some advice. thx!

Your coding style looks fine to me. The build however gave style errors ^ see the red 'X's in the build.

On your cmd line:

Run 'cargo fmt' - this neatens things automatically

Run 'cargo clippy' - this is a linter which will suggest fixes and warns of likely bugs, so you'll have to review each one.

you can see the list of errors from the online build here: https://github.com/bootandy/dust/actions/runs/9461768640/job/26334984807?pr=399

wugeer · 2024-06-19T09:01:39Z

I'm fairly new to Rust , so my coding style may not be very good. If there is anything that needs to be modified, I hope you will give me some advice. thx!

Your coding style looks fine to me. The build however gave style errors ^ see the red 'X's in the build.

On your cmd line:

Run 'cargo fmt' - this neatens things automatically

Run 'cargo clippy' - this is a linter which will suggest fixes and warns of likely bugs, so you'll have to review each one.

you can see the list of errors from the online build here: https://github.com/bootandy/dust/actions/runs/9461768640/job/26334984807?pr=399

Thank you for your guidance in helping me improve my code to meet the standards. I have resubmitted it, and all checks have passed. :)

bootandy · 2024-06-19T20:54:28Z

this method provides great flexibility, I'm concerned about whether such a high level of flexibility is necessary for this tool. Adding this feature might place a significant cognitive burden on the users. After all, people are probably accustomed to using the find command with the -mtime option, which filters by days, or the -mmin option, which filters by minutes. What are your thoughts on this? @bootandy

I don't have strong opinions on this. But lets stabilise -mtime an -mmin for now. If we want to do the more flexible one we can do that in a followup. It definitely shouldn't go in this PR.

bootandy · 2024-06-19T21:28:24Z

Would you mind doing a rebase or merge - I've accepted a different PR and there are some conflicts ^ - thanks .

wugeer · 2024-06-20T03:31:06Z

this method provides great flexibility, I'm concerned about whether such a high level of flexibility is necessary for this tool. Adding this feature might place a significant cognitive burden on the users. After all, people are probably accustomed to using the find command with the -mtime option, which filters by days, or the -mmin option, which filters by minutes. What are your thoughts on this? @bootandy

I don't have strong opinions on this. But lets stabilise -mtime an -mmin for now. If we want to do the more flexible one we can do that in a followup. It definitely shouldn't go in this PR.

Indeed, I agree with you. :)

wugeer · 2024-06-20T03:33:16Z

Would you mind doing a rebase or merge - I've accepted a different PR and there are some conflicts ^ - thanks .

Ok. I'm happy to do it! :)

…e access time, modify time, and change time of the file for statistics

bootandy · 2024-06-24T22:32:37Z

src/config.rs

 use crate::display::get_number_format;

+pub static DAY_SECEONDS: i64 = 24 * 60 * 60;


Sorry, due to my carelessness, I did not correct this spelling error in time. I will resubmit a PR for processing.

Not a problem! we all do it.

codeswhite reviewed May 26, 2024

View reviewed changes

codeswhite reviewed May 27, 2024

View reviewed changes

wugeer force-pushed the feature_xhw branch from cb97bd8 to ddc8568 Compare May 28, 2024 08:20

wugeer requested a review from codeswhite May 30, 2024 14:31

codeswhite approved these changes May 30, 2024

View reviewed changes

wugeer closed this Jun 11, 2024

wugeer reopened this Jun 11, 2024

bootandy reviewed Jun 17, 2024

View reviewed changes

bootandy requested changes Jun 17, 2024

View reviewed changes

src/dir_walker.rs Outdated

depth,

walk_data,

Copy link

Owner

bootandy Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sensible change, there were too many params.

wugeer reacted with hooray emoji

wugeer force-pushed the feature_xhw branch 2 times, most recently from 0f6e035 to 98e26ba Compare June 19, 2024 08:53

wugeer force-pushed the feature_xhw branch from bdee1fe to 1f759f5 Compare June 20, 2024 03:23

feat: Added the ability to filter the corresponding files based on th…

abc5605

…e access time, modify time, and change time of the file for statistics

wugeer force-pushed the feature_xhw branch from 1f759f5 to abc5605 Compare June 20, 2024 05:20

bootandy reviewed Jun 24, 2024

View reviewed changes

bootandy merged commit d65f410 into bootandy:master Jun 24, 2024
17 checks passed

rindeal mentioned this pull request Jun 25, 2024

Fix -x option behavior #401

Merged

bootandy mentioned this pull request Jul 16, 2024

Fix slowdown in new version #415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added the ability to filter the corresponding files based on the access_time, modify time, and change time of the file for statistics. #399

feat: Added the ability to filter the corresponding files based on the access_time, modify time, and change time of the file for statistics. #399

wugeer commented May 17, 2024

bootandy commented May 21, 2024

codeswhite May 26, 2024

wugeer May 27, 2024

codeswhite May 27, 2024

wugeer May 28, 2024

codeswhite May 27, 2024

wugeer May 28, 2024

rindeal commented May 29, 2024

codeswhite left a comment

wugeer commented May 31, 2024

wugeer commented Jun 11, 2024

bootandy commented Jun 17, 2024

bootandy Jun 17, 2024

bootandy Jun 17, 2024

wugeer Jun 18, 2024

bootandy left a comment

bootandy Jun 17, 2024

wugeer commented Jun 18, 2024

wugeer commented Jun 18, 2024

bootandy commented Jun 18, 2024

wugeer commented Jun 19, 2024

bootandy commented Jun 19, 2024

bootandy commented Jun 19, 2024

wugeer commented Jun 20, 2024

wugeer commented Jun 20, 2024

bootandy Jun 24, 2024

wugeer Jun 25, 2024

bootandy Jun 27, 2024

	'-M+[File'\''s data was last modified less than, more than or exactly <mtime>*24 hours ago. When dust figures out how many 24-hour periods ago the file was last modified, any fractional part is ignored, so to match -mtime +1, a file has to have been modified at least two days ago.]: : ' \
	'-M+[Sort by time modifed, include only files modified +/-<mtime> days before/after, days are rounded down, prefix \'\-\' for newer, \'\+\' for older. : : ' \

		use crate::display::get_number_format;

		pub static DAY_SECEONDS: i64 = 24 * 60 * 60;

feat: Added the ability to filter the corresponding files based on the access_time, modify time, and change time of the file for statistics. #399

feat: Added the ability to filter the corresponding files based on the access_time, modify time, and change time of the file for statistics. #399

Conversation

wugeer commented May 17, 2024

Background

Full Changelogs

Issue Reference

Test Result

bootandy commented May 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rindeal commented May 29, 2024

codeswhite left a comment

Choose a reason for hiding this comment

wugeer commented May 31, 2024

wugeer commented Jun 11, 2024

bootandy commented Jun 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bootandy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wugeer commented Jun 18, 2024

wugeer commented Jun 18, 2024

bootandy commented Jun 18, 2024

wugeer commented Jun 19, 2024

bootandy commented Jun 19, 2024

bootandy commented Jun 19, 2024

wugeer commented Jun 20, 2024

wugeer commented Jun 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment